Skip to content

Fix multiple results not returned with custom TestMethod and TestClass (#358)#363

Merged
jayaranigarg merged 9 commits into
microsoft:masterfrom
bignoncedric:multipletestresultsbug
Mar 28, 2018
Merged

Fix multiple results not returned with custom TestMethod and TestClass (#358)#363
jayaranigarg merged 9 commits into
microsoft:masterfrom
bignoncedric:multipletestresultsbug

Conversation

@bignoncedric

Copy link
Copy Markdown
Contributor

This PR fixes the bug #358.
In TestMethodRunner, only the first element of TestResult[] returned by Executor.Execute was used.

A test is also added to validate the RFC 003- Framework Extensibility for Custom Test Execution.

@msftclas

msftclas commented Feb 6, 2018

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

currentResult.Duration = watch.Elapsed;
foreach (var testResult in testResults)
{
testResult.DatarowIndex = rowIndex++;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DatarowIndex is used for each row of data in dataSource.
By framework extensibility you have now started running each dataRow multiple times(say 5), but we should not increase dataRowIndex for each of the 5 runs of a DataRow. It should be incremented only once per dataRow and not once per testResult.
This statement should be testResult.DataRowIndex = rowIndex;
Do rowIndex++ separately outside of the loop.

{
foreach (var test in passedTests)
{
var x = string.Join(", ", this.runEventsHandler.PassedTests.Select(_ => _.DisplayName));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this.

Assert.AreNotEqual(3, customTestClass1ExecutionCount);
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a data-driven test with [IterativeTestMethod] attribute

@jayaranigarg

Copy link
Copy Markdown
Member

Kindly sign the CLA agreement as well.

@jayaranigarg

Copy link
Copy Markdown
Member

Thank you for your contribution @bignoncedric . This PR looks awesome.
As per our offline conversation, I am keeping this PR on hold. Let me know whenever you are ready to push this in.

@jayaranigarg

Copy link
Copy Markdown
Member

@bignoncedric : Any updates here?

@federicoce-moravia

Copy link
Copy Markdown

Any updates? I'm currently sorting the ITestResult[] with a "worst outcome first" comparer before returning, but I would really like to see this handled elegantly by the library.

@ShreyasRmsft

Copy link
Copy Markdown
Member

@bignoncedric any updates?

@jayaranigarg

Copy link
Copy Markdown
Member

@dotnet-bot Test Windows / Debug Build please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants